Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add sbix fallback twemoji font (and bump to emoji 12) #3050

Merged
merged 5 commits into from Jun 3, 2019
Merged

add sbix fallback twemoji font (and bump to emoji 12) #3050

merged 5 commits into from Jun 3, 2019

Conversation

ara4n
Copy link
Member

@ara4n ara4n commented Jun 2, 2019

@ara4n ara4n requested a review from a team June 2, 2019 03:26
@ara4n ara4n changed the title add SBIX fallback twemoji font add sbix fallback twemoji font Jun 2, 2019
@ara4n
Copy link
Member Author

ara4n commented Jun 2, 2019 •
edited

The main proviso here is that ligatures aren't working for the 'family' subgroup (holding hands, kisses, hearts, family combos), and eye in speech bubbles. This is a regression over the COLR font; it's not immediately clear why. The combination is picked up as a ligature, but it's drawing the individual glyphs rather than the custom glyph defined for that ligature, despite being present.

EDIT: fixed by matrix-org/twemoji-colr@1645460

From a quick visual inspection, it's now in sync with COLR.

This was referenced Jun 2, 2019
Closed
Closed
@ara4n ara4n changed the title add sbix fallback twemoji font add sbix fallback twemoji font (and bump to emoji 12) Jun 2, 2019
turt2live
turt2live approved these changes Jun 3, 2019
View reviewed changes
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm overall

src/utils/FontManager.js
@@ -114,7 +114,13 @@ export async function fixupColorFonts() {
// weights for the emoji to appear in bold messages, etc.
document.fonts.add(new FontFace("Twemoji", path, { weight: 600 }));
document.fonts.add(new FontFace("Twemoji", path, { weight: 700 }));
} else {
// fall back to SBIX, generated via https://github.com/matrix-org/twemoji-colr/tree/matthew/sbix
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's very unfortunate that we're not able to use upstream for this. I have many fears about us forgetting to upgrade, or falling behind, or otherwise not maintaining our fork very well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, but i see no alternative sadly.

@ara4n ara4n merged commit 1515f8d into develop Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@turt2live turt2live turt2live approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@ara4n @turt2live